Skip to content

MINOR: Fix Streams Position thread-safety #19480

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 16, 2025

Conversation

cjf2xn
Copy link
Contributor

@cjf2xn cjf2xn commented Apr 16, 2025

Problem

The Position class uses multi-threaded data structures and safe
publication which suggests to users that it's thread safe. The
Position#merge method is not thread-safe due to the check-then-act
logic:

if (!partitionMap.containsKey(partition) || partitionMap.get(partition)
< offset) {
  partitionMap.put(partition, offset);
}

The partitionMap's entry for partition can change between the
conditional check and the put call.

The multi-threaded unit test reproduces the problem and fails ~80% of
the time without the fix.

Solution

Delegate mutation to withComponent which is thread-safe since it
relies on ConcurrentHashMap#compute which uses synchronization
internally to atomically execute the remapping function.

@github-actions github-actions bot added triage PRs from the community streams labels Apr 16, 2025
Copy link
Contributor

@vvcephei vvcephei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch, @cjf2xn ! Thanks for the contribution.

@vvcephei vvcephei changed the title Minor: Streams thread safe position. MINOR: Fix Streams Position thread-safety Apr 16, 2025
@vvcephei vvcephei merged commit 67fa365 into apache:trunk Apr 16, 2025
23 checks passed
@mjsax mjsax removed the triage PRs from the community label Apr 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants